Skip to content

Add HTTP2 toggling for Nginx and option for HTTP2 over HTTP#456

Merged
sroze merged 2 commits intomasterfrom
feature/http2-toggle
Feb 2, 2018
Merged

Add HTTP2 toggling for Nginx and option for HTTP2 over HTTP#456
sroze merged 2 commits intomasterfrom
feature/http2-toggle

Conversation

@andytson-inviqa
Copy link
Copy Markdown
Contributor

@andytson-inviqa andytson-inviqa commented Jan 26, 2018

Nginx already had HTTP2 enabled

Latter is disabled by default as:

  • Nginx doesn't support H2C, so is unable to negotiate HTTP version, so instead uses a different env var toggle
  • H2C isn't supported by browsers, so has limited use

@andytson-inviqa
Copy link
Copy Markdown
Contributor Author

need to double check Apache has mod_http2 enabled by default

@andytson-inviqa andytson-inviqa changed the title Add HTTP2 toggling and option for H2C (apache) or HTTP2 over HTTP (nginx) Add HTTP2 toggling for Nginx and option for HTTP2 over HTTP Jan 26, 2018
@andytson-inviqa
Copy link
Copy Markdown
Contributor Author

andytson-inviqa commented Jan 26, 2018

Turns out Ubuntu hasn't packaged mod_http2 for Apache, so I've removed the config for it

Nginx already had HTTP2 enabled

Latter is disabled by default as:
* Nginx doesn't support H2C, so is unable to negotiate HTTP version, so instead uses a different env var toggle
* H2C isn't supported by browsers, so has limited use
@sroze
Copy link
Copy Markdown
Member

sroze commented Jan 26, 2018

There is no risks for existing websites to enable HTTP/2.0, right? Also, does this mean we cannot enable HTTP2 for HTTP & HTTPS?

@andytson-inviqa
Copy link
Copy Markdown
Contributor Author

We already have HTTP2 enabled for nginx image, so any risk was already there. Enabling HTTP2 on http port though is way too risky though as it will stop http 1.1 in Nginx.

Comment thread php/nginx/README.md Outdated
WEB_HTTPS_OFFLOADED | Whether the HTTPS traffic has been forwarded without SSL to the HTTPS port | true/false | false
WEB_HTTPS_ONLY | Whether to redirect all HTTP traffic to HTTPS | true/false | $WEB_HTTPS (deprecated: if $WEB_HTTPS=true then false)
WEB_HTTP2 | Whether to enable HTTP2 over TLS on HTTPS port. If WEB_HTTPS_OFFLOADED enabled then this is ignored due to Nginx not supporting h2c | true/false | true
WEB_HTTP2_ONLY_HTTP | Whether to enable HTTP2 over plaintext on HTTP port (or HTTPS if WEB_HTTPS_OFFLOADED enabled). Nginx doesn't support h2c for plain HTTP protocol so will not support HTTP 1.1/1.0 if enabled | true/false | false
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it WEB_HTTP2_ON_HTTP ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to make it clear it would only do http2, not http 1.1, hence ONLY

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

happy to go with a different variable name though, just still keeping it clear at the same time

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WEB_HTTP2_ONLY_ON_HTTP ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WEB_HTTPS_HTTP2
WEB_HTTP_HTTP2_ONLY
?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, although that doesn't indicate that WEB_HTTPS in WEB_HTTPS_OFFLOADED enabled won't get WEB_HTTPS_HTTP2

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mind = 💥

So, we agree that this flag will configure if HTTP/2.0 is enabled on the non-secure HTTP port?
If yes, what about:

  1. WEB_HTTP2_ON_HTTP_PORT
  2. WEB_HTTP2_ON_NON_SECURE_HTTP

IMHO, we don't care about the "only" as it's in the documentation (i.e. README file)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WEB_HTTP2_TLS
WEB_HTTP2_PLAINTEXT_NONBC

@sroze sroze merged commit 4f155f6 into master Feb 2, 2018
@sroze sroze deleted the feature/http2-toggle branch February 2, 2018 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants